@claude

@claude

@claude

@claude Please do a thorough review of this PR.

Focus especially on:

• any bugs or incorrect logic in the changes
• whether these changes could impact existing flows or cause regressions
• whether the implementation is aligned with the requirement/design
• whether the changes are correct and complete
• missing edge cases, validation gaps, and test coverage gaps

Please call out concrete issues clearly and separate them by severity if possible.

AI-generated review — auto-generated by Claude Code (wallet-platform:review-pr). Please use your own judgment before acting on any findings.

PR Review: #8332 — feat(sdk-coin-sui): update balance querying to handle fundsInAddressBalance field

Author: @abhishekagrawal080 | URL: #8332
Changes: +979 / -96 across 19 files

1. Summary

This PR extends the Sui SDK to support the new fundsInAddressBalance field introduced in Sui's address-balance system (SIP-58). It updates balance querying to expose this field, and—going well beyond the ticket scope—implements full transaction-building support for spending from address balance across all transfer paths: self-pay, sponsored, SUI native, and token transfers.

Changes at a Glance

• Files: 19 changed (+979 added, -96 deleted)
• Lines: +979 / -96
• Areas: Types/Interfaces, Services, Utils, Tests, Config (sdk-core)

Dataflow Diagram

sequenceDiagram
    participant Caller
    participant Sui as sui.ts recover()
    participant Utils as utils.getBalance()
    participant RPC as suix_getBalance (Sui RPC)
    participant Builder as TransferBuilder / TokenTransferBuilder
    participant TB as TransactionBlock

    Caller->>Sui: recover(params)
    Sui->>Utils: getBalance(senderAddress)
    Utils->>RPC: suix_getBalance([owner, coinType])
    RPC-->>Utils: { totalBalance, fundsInAddressBalance }
    Utils-->>Sui: SuiBalanceInfo { totalBalance, coinObjectBalance, fundsInAddressBalance }

    alt Case 2: all funds in address balance (no coin objects)
        Sui->>Utils: getChainContext(url)
        Utils->>RPC: suix_getLatestSuiSystemState + sui_getChainIdentifier
        RPC-->>Utils: { epoch, chainId }
        Sui->>Builder: expiration({ ValidDuring: { minEpoch, maxEpoch, chain, nonce } })
    end

    Sui->>Builder: fundsInAddressBalance(amount)
    Builder->>TB: withdrawal({ amount, type })
    TB-->>Builder: BalanceWithdrawalCallArg
    Builder->>TB: moveCall("0x2::coin::redeem_funds", [withdrawal])
    TB-->>Builder: Coin<T>
    Builder->>TB: mergeCoins / splitCoins / transferObjects

Notable Changes

• getBalance() return type changed from Promise<string> to Promise<SuiBalanceInfo> — a breaking change for any external callers of the protected method.
• New BalanceWithdrawal CallArg and BCS spec added to the Mysten Labs builder layer — low-level protocol change that must be consistent with the Sui node's expected encoding.
• validateGasPayment now accepts empty array — previously enforced non-empty payment; allowing [] enables address-balance-funded gas but removes a safety check that previously caught misconfigured transactions.

2. Ticket Alignment

Ticket: CSHLD-407 — SUI: Update balance querying to handle fundsInAddressBalance field
Type: Task

Intent Match: ⚠️ PARTIAL

The ticket asks for: (1) adding fundsInAddressBalance to CoinBalance type, (2) updating getBalance() to return the field and correctly sum totals, (3) fixing recover() to treat address-balance-only addresses as spendable, and (4) unit tests. All four are implemented.

However, the PR also implements a full transaction-building layer for spending from address balance (BalanceWithdrawal PTB construction, ValidDuring expiration, sponsored paths) — work that belongs to CSHLD-409 ("withdrawal builder") referenced in the ticket. The ticket is scoped only to balance querying.

Scope Check: ⚠️ SIGNIFICANT ADDITIONS

Out-of-scope changes not described in CSHLD-407:

• Inputs.ts: New BalanceWithdrawalCallArg superstruct type + Inputs.BalanceWithdrawal() builder
• TransactionBlock.ts: New withdrawal() method
• TransactionDataBlock.ts / sui-bcs.ts: ValidDuring expiration type + full BCS encoding
• transferBuilder.ts: Three new transaction build paths (Path 1a, 1b, 2) for address balance spending
• tokenTransferBuilder.ts: Address-balance build paths for token transfers
• transaction.ts: redeem_funds MoveCall detection for transaction type classification
• utils.ts: getChainContext() RPC method
• transactionBuilder.ts: expiration() setter, empty-payment gas validation
• sdk-core/baseTypes.ts: fundsInAddressBalance on PrebuildTransactionWithIntentOptions + PopulatedIntent

These are substantive new features, not incidental cleanup.

Ticket Quality: ⚠️ MISSING DETAILS

The ticket has good context and acceptance criteria for the querying portion. However, it references CSHLD-409 for the spending/builder work without linking it — the in-scope boundary is unclear, which may have contributed to scope expansion here.

3. Index Analysis

No database queries introduced. This is a pure TypeScript SDK module — MongoDB not applicable.

4. Type Safety Audit

Issue 1: any() in BalanceWithdrawalCallArg superstruct schema

File: modules/sdk-coin-sui/src/lib/mystenlab/builder/Inputs.ts:67-69

export const BalanceWithdrawalCallArg = object({
  BalanceWithdrawal: object({
    amount: any(),   // 🚨
    type_: any(),    // 🚨
  }),
});

Risk: any() disables superstruct validation for these fields entirely. amount should be union([integer(), bigint()]) and type_ should mirror the existing TypeTag schema. If a caller passes a string for amount, it won't be caught until BCS serialization (which encodes it as BCS.U64 and may silently truncate or error at runtime).

Fix: Use concrete superstruct validators or at minimum add a note explaining why any() is required here.

Issue 2: as any[] casts when scanning inputs for BalanceWithdrawal

File: modules/sdk-coin-sui/src/lib/tokenTransferBuilder.ts:317 and transferBuilder.ts:698

const withdrawalInput = (tx.suiTransaction?.tx?.inputs as any[])?.find(
  (input: any) =>
    (input !== null && typeof input === 'object' && 'BalanceWithdrawal' in input) ||
    (input?.value !== null && typeof input?.value === 'object' && 'BalanceWithdrawal' in (input.value ?? {}))
);

Risk: Casting inputs to any[] bypasses typing for the entire array. The input: any callback parameter then propagates through the bw.amount read. If the input shape changes, this will fail silently at the new BigNumber(String(bw.amount)) call.

Fix: Use BuilderCallArg (or the union CallArg | TransactionBlockInput) as the element type and narrow with a type guard:

function isBalanceWithdrawalInput(input: unknown): input is { BalanceWithdrawal: { amount: bigint | number; type_: TypeTag } } {
  return typeof input === 'object' && input !== null && 'BalanceWithdrawal' in input;
}

Issue 3: tx as any in transaction type detection

File: modules/sdk-coin-sui/src/lib/transaction.ts:566

const moveCall = tx as any;
if (!moveCall.target?.endsWith(MethodNames.RedeemFunds)) return false;
const typeArg: string = moveCall.typeArguments?.[0] ?? '';

Risk: tx is already narrowed to kind === 'MoveCall' at this point. The MoveCall transaction type should have target and typeArguments fields — using as any hides whether this narrowing is actually typed correctly in the codebase.

Fix: Cast to the specific MoveCallTransaction type instead of any.

Issue 4: as SuiCoin cast without type guard

File: modules/sdk-coin-sui/src/lib/tokenTransferBuilder.ts:292

private get tokenCoinType(): string {
  const config = this._coinConfig as SuiCoin;
  return `${config.packageId}::${config.module}::${config.symbol}`;
}

Risk: _coinConfig is typed as Readonly<CoinConfig> (base class). If this builder is ever instantiated with a non-SUI token config that lacks packageId, tokenCoinType silently returns undefined::undefined::undefined, which will produce an invalid Move type argument passed to redeem_funds — a runtime failure with no helpful error message.

Fix: Add a guard or assertion:

if (!('packageId' in config)) throw new Error('TokenTransferBuilder requires a SuiCoin config');

Issue 5: getIdFromCallArg returns '' for BalanceWithdrawal

File: modules/sdk-coin-sui/src/lib/mystenlab/builder/Inputs.ts:98-105

if ('BalanceWithdrawal' in arg) {
  // BalanceWithdrawal inputs have no object ID; they cannot be deduplicated by ID
  return '';
}

Risk: If a transaction contains two BalanceWithdrawal inputs (not expected in practice, but possible), both would return '' from getIdFromCallArg. If this is used downstream for deduplication, both would be treated as the same input. The comment acknowledges this but returning '' rather than something unique (or throwing) may cause subtle bugs.

Issue 6: Negative coinObjectBalance not guarded

File: modules/sdk-coin-sui/src/lib/utils.ts:954-955

const coinObjectBalance = new BigNumber(totalBalance).minus(new BigNumber(fundsInAddressBalance)).toString();

Risk: If the RPC response ever returns fundsInAddressBalance > totalBalance (a node bug or race condition), coinObjectBalance becomes negative. This won't be caught and will propagate into balance calculations.

Fix: Clamp to zero: BigNumber.max(0, new BigNumber(totalBalance).minus(...)).

Issue 7: CoinBalance superstruct uses number() for balance fields

File: modules/sdk-coin-sui/src/lib/mystenlab/types/coin.ts:176-177

totalBalance: number(),
fundsInAddressBalance: optional(number()),

Risk: Sui's u64 values can exceed Number.MAX_SAFE_INTEGER (2^53 - 1 ≈ 9 × 10^15 MIST ≈ 9,000 SUI). Using number() will silently lose precision for large balances. The ticket explicitly notes the field is "a u64 returned as string by the RPC" — string() is the correct superstruct type (matching how the existing RPC layer handles it with .toString()).

Flag Summary

Generated by [wallet-platform:review-pr] via Claude Code

@abhishekagrawal080 Is there any staking related changes that's required?